Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dialog memory handling #1899

Merged
merged 7 commits into from
May 28, 2024
Merged

Improve dialog memory handling #1899

merged 7 commits into from
May 28, 2024

Conversation

vkbo
Copy link
Owner

@vkbo vkbo commented May 27, 2024

Summary:

This PR:

  • Tackles some potential issues with deleting Qt objects. It changes the split and merge dialog classes to use class methods instead, similar to other pop-up dialogs. Then it can call the deleteLater function for the C++ side after all data is processed and collected, as opposed to before when it was called on close. I am a little uncertain exactly when the delete would happen in the latter case, but I haven't had any bug reports on crashes when using these tools. Still, I think this implementation is safer.
  • Adds a custom NDialog class to serve as an in-between class for QDialog. Currently, all it adds is an overload of keyPressEvent to capture Escape key presses and redirect them to close(). The default behaviour of Qt hides the dialog without going through close, thus bypassing the closeEvent logic.
  • Replaces all QDialog.deleteLater() calls with a new NDialog.softDelete() method that simply sets self.setParent(None). This seems to play better together with the Python garbage collector. The Qt object does not seem to be deleted before it is released by the Python side.
  • Remove deleteLater() calls on QMessageBox objects. They seem to be deleted on close already.

Related Issue(s):

Reviewer's Checklist:

  • The header of all files contain a reference to the repository license
  • The overall test coverage is increased or remains the same as before
  • All tests are passing
  • All flake8 checks are passing and the style guide is followed
  • Documentation (as docstrings) is complete and understandable
  • Only files that have been actively changed are committed

@vkbo vkbo added this to the Release 2.5 RC 1 milestone May 27, 2024
@vkbo vkbo merged commit b387150 into main May 28, 2024
8 checks passed
@vkbo vkbo deleted the dialog_handling branch May 28, 2024 21:19
@vkbo vkbo changed the title Improve dialog handling Improve dialog memory handling May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant